Skip to content

Use setdefault for internal cache to ensure identity caching#2217

Merged
seberg merged 1 commit into
NVIDIA:mainfrom
seberg:ft-fixes-setdefault
Jun 15, 2026
Merged

Use setdefault for internal cache to ensure identity caching#2217
seberg merged 1 commit into
NVIDIA:mainfrom
seberg:ft-fixes-setdefault

Conversation

@seberg

@seberg seberg commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Switch selected cache write sites to setdefault/get-based patterns and keep explicit comments where setdefault is intentionally not used for primitive value caches.
(I just thought having setdefault mentioned might be nice in case of cargo-culting.)

Towards gh-2194; pytest-run-parallel does have a specific test for the GraphMemoryResource change. I could add an identity test for ComputeCapability probably (I doubt it actually matters for it, the use of setdefault just feels like a good pattern to me).

@copy-pr-bot

copy-pr-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@seberg seberg self-assigned this Jun 15, 2026
@seberg seberg added the bug Something isn't working label Jun 15, 2026
@github-actions github-actions Bot added the cuda.core Everything related to the cuda.core module label Jun 15, 2026
@seberg seberg added P0 High priority - Must do! and removed cuda.core Everything related to the cuda.core module labels Jun 15, 2026
@seberg seberg changed the title Use setdefault-oriented cache updates in core hot paths Use setdefault for internal cache to ensure identity caching Jun 15, 2026
@seberg
Use setdefault-oriented cache updates in core hot paths
3497ccf 
Switch selected cache write sites to setdefault/get-based patterns and
keep explicit comments where setdefault is intentionally not used for
primitive value caches.

Signed-off-by: Sebastian Berg <sebastianb@nvidia.com>
@seberg seberg force-pushed the ft-fixes-setdefault branch from e30c476 to 3497ccf Compare June 15, 2026 10:21
@github-actions github-actions Bot added the cuda.core Everything related to the cuda.core module label Jun 15, 2026
@seberg seberg added this to the cuda.core v1.1.0 milestone Jun 15, 2026
@seberg seberg added cuda.core Everything related to the cuda.core module and removed cuda.core Everything related to the cuda.core module labels Jun 15, 2026
@seberg

seberg commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

/ok to test 3497ccf

@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown
Doc Preview CI

🚀 View preview at
https://nvidia.github.io/cuda-python/pr-preview/pr-2217/

https://nvidia.github.io/cuda-python/pr-preview/pr-2217/cuda-core/

https://nvidia.github.io/cuda-python/pr-preview/pr-2217/cuda-bindings/

https://nvidia.github.io/cuda-python/pr-preview/pr-2217/cuda-pathfinder/


Preview will be ready when the GitHub Pages deployment is complete.

mdboom
mdboom approved these changes Jun 15, 2026
View reviewed changes

@mdboom mdboom left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Do we want to add any AGENTS.md content about this pattern being preferred over functools.cache or the non-setdefault caching pattern?

@seberg

seberg commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

Do we want to add any AGENTS.md content about this pattern being preferred over functools.cache or the non-setdefault caching pattern?

Not sure AGENTS.md has such code styles yet? But if we want to add that I can follow-up.
FWIW, in most cases @functools.cache is nicer, but if it matters to return an idempotent result strictly then it doesn't (at the moment).

However, @rwgk: When scanning for @cache I didn't modify cuda-pathfinder since almost all places were just strings, etc. where it is good.
The one odd-one-out that caught my eye is load_nvidia_dynamic_lib if loading twice from two threads or the function being strictly idempotent can matter, then we need to fix it (I can do that in a follow-up if it seems important to you.)

@seberg seberg merged commit 3833b93 into NVIDIA:main Jun 15, 2026
202 of 206 checks passed
github-actions Bot pushed a commit that referenced this pull request Jun 16, 2026
Clean up PR preview folders for 4 closed/merged PRs
5e91eee 
Removed preview folders for the following PRs:
- PR #2150
- PR #2217
- PR #2221
- PR #2224
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mdboom mdboom mdboom approved these changes

Assignees

@seberg seberg

Labels

bug Something isn't working cuda.core Everything related to the cuda.core module P0 High priority - Must do!

Projects

None yet

Milestone

cuda.core v1.1.0

Development

Successfully merging this pull request may close these issues.

2 participants

@seberg @mdboom